-
Notifications
You must be signed in to change notification settings - Fork 581
fix openapi spec import for specific urls #907
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
|
Added a workaround in |
|
@Udhay-Adithya Can you add links to more real world openAPI spec URLs for which our parser works or doesn't works? |
APIDash - Works without any fix |
|
It’s not our parser that causes the issue. The failures come from the documents themselves and how the openapi_spec package (correctly) enforces OpenAPI shapes. Valid security fields work fine as per the package docs; the broken cases are due to invalid spec content. Findings per document
Steps to reproduce failures from local files, How to run
Expected outcomes
|
|
@Udhay-Adithya these comments are insightful. I will create a dev doc adding your comments and merge this PR. THank you. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR fixes OpenAPI spec import functionality for URLs containing specifications with empty security arrays, which previously failed due to a bug in the openapi_spec package. The solution implements a workaround that temporarily removes problematic security fields during parsing.
- Adds a workaround for openapi_spec package parsing issues with empty security arrays
- Implements preprocessing logic to detect and handle problematic security fields
- Adds comprehensive test coverage for the new parsing scenarios
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| lib/dashbot/services/openapi_import_service.dart | Implements workaround logic with preprocessing methods to handle empty security arrays |
| test/dashbot/services/openapi_import_service_test.dart | Adds test cases covering the new parsing scenarios and edge cases |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| } catch (e) { | ||
| // Try workaround for security field parsing issues | ||
| try { | ||
| final processedSource = _removeProblematicSecurityField(source); | ||
| if (processedSource != source) { | ||
| return OpenApi.fromString(source: processedSource, format: null); | ||
| } | ||
| } catch (_) { | ||
| // Workaround failed, fall through to return null | ||
| } | ||
| return null; | ||
| } |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The workaround logic has a potential issue: if _removeProblematicSecurityField throws an exception, it's caught and ignored, but the method returns null instead of re-throwing the original parsing exception. This could mask the actual parsing error. Consider logging the original exception or providing more specific error handling.
|
|
||
| return source; | ||
| } catch (e) { | ||
| throw FormatException('Failed to preprocess OpenAPI spec: $e'); |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The error message could be more specific about what preprocessing failed. Consider including details about the security field processing or JSON parsing failure to help with debugging.
|
|
||
| /// Removes problematic security fields that cause parsing issues. | ||
| /// TODO: Remove this workaround once openapi_spec package fixes | ||
| /// the issue with security fields containing empty arrays. |
Copilot
AI
Oct 11, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The TODO comment should include a reference to track when this workaround can be removed, such as a GitHub issue number or version number of the openapi_spec package that will contain the fix.
| /// the issue with security fields containing empty arrays. | |
| /// the issue with security fields containing empty arrays. | |
| /// See: https://github.com/omidyar/openapi_spec/issues/123 (update with actual issue number) |
|
@copilot Create a openAPI spec document in |
PR Description
The OpenAPI import feature was failing when trying to import specifications from URLs like
https://catfact.ninja/docs?api-docs.json. The error "The fetched content does not look like a valid OpenAPI spec (JSON or YAML)" was shown even though the content was a valid OpenAPI 3.0 specification. This was caused by a bug in theopenapi_specpackage (v0.15.0) that cannot parse OpenAPI specs containing"security": [[]](empty security arrays), which is valid according to the OpenAPI 3.0 specification.Related Issues
Checklist
mainbranch before making this PRflutter upgradeand verify)flutter test) and all tests are passingAdded/updated tests?
OS on which you have developed and tested the feature?